fix(api): make WatchSandbox loss-aware and resumable by letv1nnn · Pull Request #3209 · NVIDIA/OpenShell

letv1nnn · 2026-09-06T20:17:05Z

Summary

Make WatchSandbox loss-aware and resumable. Recoverable broadcast lag now emits a SandboxStreamWarning and keeps streaming instead of terminating with RESOURCE_EXHAUSTED. Every resumable event (log line, platform event) carries a monotonic per-sandbox cursor, and a reconnecting client can pass resume_after_cursor to replay only what it missed. Unrecoverable loss (a trimmed cursor) terminates the stream with OUT_OF_RANGE so gaps are never silent.

Draft / work in progress, server, proto, docs, and the Rust SDK reconnect helper are complete; Go/TS/Python SDK helpers and e2e coverage are still pending (see TODO).

Related Issue

Refs #3055 (partial; issue stays open until SDK helpers land).

Changes

Lag is recoverable. Status, log, and platform receivers emit SandboxStreamWarning and continue on RecvError::Lagged; Closed stays terminal. Adds lag_warning / lag_warning_event helpers.
Shared cursor. TracingLogBus and PlatformEventBus draw sequence numbers from one per-sandbox SeqAllocator, so the merged stream is ordered in a single cursor space. Each buffered event is stamped at publish time.
Proto. Adds SandboxStreamEvent.cursor and WatchSandboxRequest.resume_after_cursor. cursor = 0 marks non-resumable events (status snapshots, warnings); resume_after_cursor = 0 means no cursor resume (tail-limited replay via log_tail_lines / event_tail).
Resume + gap detection. tail_after(cursor) returns events after a cursor or a ResumeGap when the requested cursor was trimmed. The producer replays both resumable sources after the cursor, merged in cursor order, then enters live delivery. A gap terminates with OUT_OF_RANGE carrying the requested and earliest-available cursors.
Exactly-once at the replay/live boundary. The producer tracks the highest replayed cursor and suppresses live events at or below it, so an event buffered during watch initialization is delivered once.
Teardown. TracingLogBus::remove clears both resumable per-sandbox maps before resetting the shared allocator, closing a publish-during-teardown window.
Docs. Documents the cursor contract, recoverable vs unrecoverable loss, and cross-source ordering in proto, architecture/gateway.md, and docs/observability/accessing-logs.mdx.
Rust SDK resume helper. OpenShellClient::watch_logs (and the workspace-scoped variant) yield a typed WatchEvent stream that tracks the highest cursor, reconnects on transient errors with capped backoff, resends resume_after_cursor to replay only missed events, forwards Warning events, and terminates on OUT_OF_RANGE (new SdkError::OutOfRange). Backoff resets on each delivered event.

TODO (follow-up)

#	PR	Scope
A	#3613	Addresses @varshaprasad96's interleaving concern directly: `snapshot_after` reads both buses under one lock instead of two independent `tail_after` calls, closing the race window. Also adds a coverage floor so a source with a shallow tail limit (e.g. default `event_tail=0`) can't be silently skipped while a deeper source's cursor is handed out as safe to resume from.
B	#3662	Cursor-tracking watch helper for Go, mirroring the Rust SDK's reconnect/backoff/warning/`OUT_OF_RANGE` semantics from `crates/openshell-sdk/src/client.rs`.
C	feat(ts-sdk): curated watch	Same contract, `sdk/typescript/src/client.ts`.
D	#3711	Same contract, `python/openshell/sandbox.py`.
E	test(e2e): watch resume	New `e2e/rust/tests/sandbox_watch_resume.rs` covering reconnect-no-loss, cursor expiry → `OUT_OF_RANGE`, and the `stop_on_terminal` ERROR regression.

A lands first since B/C/D/E all build against its resume contract. B/C/D are independent of each other once A merges.

Testing

cargo test -p openshell-server — all pass, including resume replay, cross-source merge order, duplicate suppression, gap → OUT_OF_RANGE, init-race single-delivery, and lag-warning regression.
mise run pre-commit
E2E — pending SDK helpers.

Checklist

Follows Conventional Commits
Commits are signed off (DCO)
Architecture docs updated
SDK helpers (tracked above)

copy-pr-bot · 2026-09-06T20:17:08Z

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

johntmyers · 2026-09-09T19:45:20Z

gator-agent

Blocked

Gator is blocked because PR #3209 currently has merge conflicts with main.

Next action: @letv1nnn, please rebase or merge the latest main into this branch, resolve the conflicts, and push the updated branch. Gator will review the new head and dispatch the authorized E2E workflow once the conflict is cleared and review feedback is resolved.

Gator metadata

Head SHA: 276a7c14af4d8ab1c1368a60922567e9c8c28a1f
Base SHA: 1510e2c5a77903f9ff06a1317e142abad139a9b2
Merge base SHA: 320d4ef79dd572c642133f175f12bafc20d89fd9
Patch ID: adafa0188b960606855fc49415c4f8a3eaa6f007
Gator payload: 8
Next state: gator:blocked
Blocked reason: merge_conflict

johntmyers

gator-agent

PR Review Status

The merge-conflict blocker is cleared, and the initial code review found two cursor-resume paths that can silently omit events.

Action required: @letv1nnn, please make cursor-space resets unambiguous and make initial cross-source replay safe for reconnect watermarks, including the deterministic regression cases in the inline findings.

Blocking findings:

GATOR-289481f5-01: stale cursors can be accepted in a replacement cursor space
GATOR-289481f5-02: initial replay can acknowledge an undelivered lower cursor

Carried findings:

None

Gator metadata

Validation: Project-valid implementation of linked issue #3055
Docs: Fern docs updated in docs/observability/accessing-logs.mdx; the cursor-space claim needs to remain aligned with the fix
Checks: DCO passes; required branch checks have not been dispatched for this current head
E2E: test:e2e required for gateway, sandbox-watch, and SDK reconnect behavior; deferred until blocking review feedback is resolved
Head SHA: 289481f5aa5f1cbd856045f052968017bcb2aeff
Base SHA: bcf96e4900004e07253d72332a6619f7e2c4e7d7
Merge base SHA: bcf96e4900004e07253d72332a6619f7e2c4e7d7
Patch ID: b7682c23e96f9512168ee19074a25ae4063f4e72
Gator payload: 8
Review mode: initial
Previous reviewed SHA: none
Review budget exhausted: no
Maintainer decision required: no
Next state: gator:in-review

johntmyers · 2026-09-14T21:14:07Z

gator-agent

PR Review Status

Thanks @letv1nnn. The new opaque, epoch-bound cursor fixes stale cursors being accepted after a completed reset, and the merged replay fixes the lower-cursor omission across initial tail sources. I resolved GATOR-289481f5-02 after checking the new replay ordering and source-specific cutoffs.

One carried obligation remains: cursor-space validation and replay collection happen under separate locks. If teardown retires the validated space and a publisher creates a replacement space before the tail reads, the old sequence can be applied to the replacement buffers and silently omit their lower events.

Action required: make epoch validation and both replay snapshots atomic with respect to cursor-space replacement, or revalidate the epoch after collecting both tails and before emitting any replay event; add a deterministic teardown/republish race regression.

Blocking findings:

No new findings

Carried findings:

GATOR-289481f5-01: still open — a concurrent cursor-space replacement between validation and replay can mix epochs and skip replacement-space events

Gator metadata

Validation: Project-valid implementation of issue fix(api): make WatchSandbox loss-aware and resumable #3055
Docs: Fern and architecture docs are updated and aligned with the intended cursor-space contract
Checks: DCO passes; required branch checks are pending dispatch for this head
E2E: test:e2e remains required for gateway, sandbox-watch, and SDK reconnect behavior; dispatch is deferred until the carried blocker is resolved
Head SHA: 076caddc519a40420171443aeae1edb9aea99f73
Base SHA: cc780d4e17512e8f804f68c153c033fe6584ae3a
Merge base SHA: cc780d4e17512e8f804f68c153c033fe6584ae3a
Patch ID: d11f086e61c1a3de47937d065cd02c69a23a8103
Gator payload: 8
Review mode: follow_up
Previous reviewed SHA: 289481f5aa5f1cbd856045f052968017bcb2aeff
Review budget exhausted: no
Maintainer decision required: no
Next state: gator:in-review

github-actions · 2026-09-14T22:55:36Z

Label test:e2e applied, but pull-request/3209 does not exist yet. A maintainer needs to comment /ok to test 5cca5aa67c40f8a9470c95920e69f74a9b707080 to mirror this PR. Once the mirror exists, re-apply the label or re-run Branch E2E Checks from the Actions tab.

johntmyers · 2026-09-14T22:55:49Z

/ok to test 5cca5aa

johntmyers · 2026-09-14T22:56:25Z

gator-agent

PR Review Status

Thanks @letv1nnn. I checked the new post-replay epoch validation against teardown, replacement-space publication, both replay snapshots, and the no-await producer path. It closes the carried cursor-space race, and the independent follow-up review found no new blocking defects.

Blocking findings:

No blocking findings remain

Carried findings:

None; GATOR-289481f5-01 is resolved

Gator metadata

Validation: Project-valid implementation of issue fix(api): make WatchSandbox loss-aware and resumable #3055
Docs: Fern and architecture docs are updated for the opaque cursor and second epoch validation
Checks: DCO passes; current-head Branch Checks and Helm Lint are queued or running
E2E: test:e2e applied; /ok to test 5cca5aa67c40f8a9470c95920e69f74a9b707080 created the mirror; Branch E2E run 34906474483 is queued
Head SHA: 5cca5aa67c40f8a9470c95920e69f74a9b707080
Base SHA: cc780d4e17512e8f804f68c153c033fe6584ae3a
Merge base SHA: cc780d4e17512e8f804f68c153c033fe6584ae3a
Patch ID: 9ac1e3abffc995dd86fb510d227b2868cc4b445f
Gator payload: 8
Review mode: follow_up
Previous reviewed SHA: 076caddc519a40420171443aeae1edb9aea99f73
Review budget exhausted: no
Maintainer decision required: no
Next state: gator:watch-pipeline

johntmyers · 2026-09-15T00:40:20Z

@letv1nnn please address open comments so we can get this merge ready

Add tail_after() to the log and platform event buses, returning every buffered event newer than a client's resume cursor. Each PerSandbox now tracks last_trimmed_seq (the highest seq it has evicted) so a resume is reported as an unrecoverable ResumeGap only when this bus dropped an event the client still needs. Judging gaps by evictions, not by the tail's oldest seq, is required under the shared cursor space: each bus's tail is non-contiguous in the global sequence because the other bus owns the missing seqs, so comparing against tail.front() would flag false gaps. Signed-off-by: Artem Lytvyn <alytvyn@redhat.com>

… buses Wire resume_after_cursor into the watch producer. On a non-zero cursor, replay events strictly after it from both the log and platform buses, merge by shared cursor, and emit in order before entering the live loop. A trimmed range on either bus is an unrecoverable gap and terminates the stream with OUT_OF_RANGE carrying the requested and earliest-available cursors, distinct from recoverable lag which warns and continues. Signed-off-by: Artem Lytvyn <alytvyn@redhat.com>

Add handler-level tests for the resumable watch stream: replay strictly after the client cursor, merge log and platform events in shared-cursor order, suppress duplicates when resuming at the latest cursor, and terminate with OUT_OF_RANGE when the requested cursor has been trimmed. Signed-off-by: Artem Lytvyn <alytvyn@redhat.com>

Signed-off-by: Artem Lytvyn <alytvyn@redhat.com>

… cursor Signed-off-by: Artem Lytvyn <alytvyn@redhat.com>

…it the watch lag warning before its batch Signed-off-by: Artem Lytvyn <alytvyn@redhat.com>

…nitialization Signed-off-by: Artem Lytvyn <alytvyn@redhat.com>

Signed-off-by: Artem Lytvyn <alytvyn@redhat.com>

Signed-off-by: John Myers <9696606+johntmyers@users.noreply.github.com>

johntmyers · 2026-09-22T18:28:11Z

/ok to test 2cd5f15

johntmyers

gator-agent

PR Review Status

The latest rebase reconciliation preserves the reviewed watch-resume work, but it moves main's bounded relay cleanup from interactive exec to the non-interactive path. An interactive command can now finish while its RPC waits indefinitely for a relay peer that remains open, preventing the terminal event from reaching the client.

Action required: restore bounded proxy cleanup on every interactive-exec completion path, while retaining it on non-interactive exec only if that path also requires the bound.

Blocking findings:

GATOR-2cd5f15b-01: interactive relay cleanup can wait forever after command completion

Carried findings:

None; GATOR-289481f5-01, GATOR-289481f5-02, and GATOR-b17b5f39-01 remain resolved

Gator metadata

Validation: Project-valid partial implementation of issue #3055
Docs: Fern, architecture, and protobuf docs cover the resumable cursor contract
Checks: DCO passes; current-head Branch Checks and Helm Lint are queued or running
E2E: test:e2e is applied; /ok to test 2cd5f15b9411b4871ad92f44b0929d1accfc6241 created the mirror; Branch E2E is queued or running
Head SHA: 2cd5f15b9411b4871ad92f44b0929d1accfc6241
Base SHA: 718dba34304421af9d5e3ac4f9fea148ba08e0fb
Merge base SHA: 718dba34304421af9d5e3ac4f9fea148ba08e0fb
Patch ID: 422d8aeef8f43a30dde125208ca155dd267d01df
Gator payload: 9
Review mode: critical_only
Previous reviewed SHA: 55480b1df47aaf5d69bdc9c0c14273fcfa8f512f
Review budget exhausted: yes
Maintainer decision required: no
Next state: gator:in-review

Signed-off-by: John Myers <9696606+johntmyers@users.noreply.github.com>

johntmyers · 2026-09-22T18:55:53Z

/ok to test d49163e

johntmyers

gator-agent

PR Review Status

The latest delta restores bounded relay cleanup on the interactive exec timeout, error, and normal-completion paths, resolving GATOR-2cd5f15b-01. The bounded critical-only review found no newly introduced Critical defect, so no blocking findings remain.

Blocking findings:

No blocking findings remain

Carried findings:

None; GATOR-289481f5-01, GATOR-289481f5-02, GATOR-b17b5f39-01, and GATOR-2cd5f15b-01 remain resolved

Gator metadata

Validation: Project-valid partial implementation of issue #3055
Docs: Fern, architecture, and protobuf docs cover the resumable cursor contract
Checks: DCO, Branch Checks, and Helm Lint pass; current-head E2E is running
E2E: test:e2e is applied; /ok to test d49163e0501e4d543f14741ccb15647d7d63f102 created the mirror and Branch E2E is running
Head SHA: d49163e0501e4d543f14741ccb15647d7d63f102
Base SHA: 718dba34304421af9d5e3ac4f9fea148ba08e0fb
Merge base SHA: 718dba34304421af9d5e3ac4f9fea148ba08e0fb
Patch ID: bec9bb534dc7b657ef895f7b9aaca40e661af53e
Gator payload: 9
Review mode: critical_only
Previous reviewed SHA: 2cd5f15b9411b4871ad92f44b0929d1accfc6241
Review budget exhausted: yes
Maintainer decision required: no
Next state: gator:watch-pipeline

johntmyers · 2026-09-22T20:08:00Z

gator-agent

Monitoring Complete

Monitoring is complete because this PR has merged.

Final status: The last Gator state was gator:merge-ready; blocking review findings were resolved and maintainer approval was present.

I removed the active gator:* label because there is nothing left for gator to monitor on this PR.

letv1nnn force-pushed the 3055-watch-resumable-cursor/letv1nnn branch from 99d95d9 to 1f35df9 Compare September 7, 2026 14:43

letv1nnn marked this pull request as ready for review September 7, 2026 15:10

letv1nnn requested review from a team, derekwaynecarr, mrunalp and sjenning as code owners September 7, 2026 15:10

johntmyers added the gator:blocked Gator is blocked by process or repository gates label Sep 9, 2026

varshaprasad96 reviewed Sep 10, 2026

View reviewed changes

Comment thread crates/openshell-server/src/grpc/sandbox.rs

Comment thread crates/openshell-server/src/grpc/sandbox.rs Outdated

johntmyers reviewed Sep 10, 2026

View reviewed changes

Comment thread crates/openshell-server/src/grpc/sandbox.rs Outdated

Comment thread crates/openshell-server/src/grpc/sandbox.rs Outdated

johntmyers added gator:in-review Gator is reviewing or awaiting PR review feedback and removed gator:blocked Gator is blocked by process or repository gates labels Sep 10, 2026

letv1nnn requested review from johntmyers and varshaprasad96 September 14, 2026 18:51

johntmyers added the test:e2e Requires end-to-end coverage label Sep 14, 2026

johntmyers added gator:in-review Gator is reviewing or awaiting PR review feedback and removed gator:watch-pipeline Gator is monitoring PR CI/CD status gator:in-review Gator is reviewing or awaiting PR review feedback labels Sep 15, 2026

letv1nnn and others added 17 commits September 22, 2026 11:26

docs(api): document WatchSandbox loss-awareness and resume

b27cb2e

Signed-off-by: Artem Lytvyn <alytvyn@redhat.com>

fix(server): deliver watch events once and harden cursor teardown

5d6be62

Signed-off-by: Artem Lytvyn <alytvyn@redhat.com>

feat(sdk): add loss-aware resumable watch_logs to Rust SDK client

f52e558

Signed-off-by: Artem Lytvyn <alytvyn@redhat.com>

fix(server): keep watch cursors monotonic across teardown and restart

9e32f4f

Signed-off-by: Artem Lytvyn <alytvyn@redhat.com>

fix(server): merge live watch sources by cursor before emission

b21b043

Signed-off-by: Artem Lytvyn <alytvyn@redhat.com>

fix(api): bind watch cursors to a cursor space and merge tail sources

f1b25eb

Signed-off-by: Artem Lytvyn <alytvyn@redhat.com>

fix(server): revalidate the watch cursor space after collecting replay

51c2c0f

Signed-off-by: Artem Lytvyn <alytvyn@redhat.com>

test(server): update the public RPC schema fingerprint for the string…

50d7d7b

… cursor Signed-off-by: Artem Lytvyn <alytvyn@redhat.com>

fix(server): hold watch events above the publication watermark and em…

cdc9c35

…it the watch lag warning before its batch Signed-off-by: Artem Lytvyn <alytvyn@redhat.com>

test(server): synchronize the watch live-order test with the end of i…

85cdb09

…nitialization Signed-off-by: Artem Lytvyn <alytvyn@redhat.com>

fix(sdk): use canonical sandbox name in watch_logs

1cd7406

Signed-off-by: Artem Lytvyn <alytvyn@redhat.com>

test(sdk): guard canonical-name addressing in watch_logs

e7cb5db

Signed-off-by: Artem Lytvyn <alytvyn@redhat.com>

fix(server): fix public rpc schema

64d8976

Signed-off-by: Artem Lytvyn <alytvyn@redhat.com>

fix(api): reconcile watch resume rebase

2cd5f15

Signed-off-by: John Myers <9696606+johntmyers@users.noreply.github.com>

johntmyers reviewed Sep 22, 2026

View reviewed changes

Comment thread crates/openshell-server/src/grpc/sandbox.rs Outdated

fix(server): bound interactive relay cleanup

d49163e

Signed-off-by: John Myers <9696606+johntmyers@users.noreply.github.com>

johntmyers reviewed Sep 22, 2026

View reviewed changes

johntmyers approved these changes Sep 22, 2026

View reviewed changes

Conversation

letv1nnn commented Sep 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Related Issue

Changes

TODO (follow-up)

Testing

Checklist

Uh oh!

copy-pr-bot Bot commented Sep 6, 2026

Uh oh!

johntmyers commented Sep 9, 2026

Blocked

Uh oh!

Uh oh!

Uh oh!

johntmyers left a comment

Choose a reason for hiding this comment

PR Review Status

Uh oh!

Uh oh!

Uh oh!

johntmyers commented Sep 14, 2026

PR Review Status

Uh oh!

github-actions Bot commented Sep 14, 2026

Uh oh!

johntmyers commented Sep 14, 2026

Uh oh!

johntmyers commented Sep 14, 2026

PR Review Status

Uh oh!

johntmyers commented Sep 15, 2026

Uh oh!

johntmyers commented Sep 22, 2026

Uh oh!

johntmyers left a comment

Choose a reason for hiding this comment

PR Review Status

Uh oh!

Uh oh!

johntmyers commented Sep 22, 2026

Uh oh!

johntmyers left a comment

Choose a reason for hiding this comment

PR Review Status

Uh oh!

johntmyers commented Sep 22, 2026

Monitoring Complete

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

letv1nnn commented Sep 6, 2026 •

edited

Loading